Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated open graph images for blog posts #582

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

sjoerdbeentjes
Copy link
Contributor

@sjoerdbeentjes sjoerdbeentjes commented Mar 1, 2023

@sjoerdbeentjes sjoerdbeentjes changed the title base for og images og images Mar 1, 2023
@sjoerdbeentjes sjoerdbeentjes force-pushed the og-images branch 6 times, most recently from 0cc1090 to 3a611cf Compare March 1, 2023 16:27
@sjoerdbeentjes sjoerdbeentjes marked this pull request as ready for review March 2, 2023 09:27
@sjoerdbeentjes sjoerdbeentjes requested a review from Siilwyn March 2, 2023 09:29
@sjoerdbeentjes sjoerdbeentjes changed the title og images Automated open graph images for blog posts Mar 2, 2023
"deno.enable": true,
"deno.unstable": true,
"deno.enablePaths": ["netlify/edge-functions"]
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit anti-pragmatic, but I do not like to check in editor specific config files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add them to .gitignore 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 I meant your global .gitignore

netlify.toml Outdated Show resolved Hide resolved
netlify/edge-functions/og.tsx Show resolved Hide resolved
netlify/edge-functions/og.tsx Outdated Show resolved Hide resolved

const { title, image, authors } = openGraphImage

const url = `/api/og?title=${title}${image ? `&imageUrl=${image.url}` : ''}${authors.length ? `&authors=${JSON.stringify(authors)}` : ''}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use withQuery from ufo to create this URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't seem to handle json very well, so it became this: https://github.com/voorhoede/voorhoede-website/pull/582/files#diff-d638ef46d91b5f3782a9f6bf0bbaf9b4d0806449ba02b60737bc839d79251f84R10-R13

Open for suggestions on how to fix this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah maybe we shouldn't pass JSON in a query parameter at all?

@@ -0,0 +1,18 @@
export function useOpenGraphImage(openGraphImage) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this composable code to useSeoHead.

package.json Outdated Show resolved Hide resolved
@sjoerdbeentjes sjoerdbeentjes force-pushed the og-images branch 2 times, most recently from 91c6f68 to 8214925 Compare March 8, 2023 15:12
@sjoerdbeentjes sjoerdbeentjes requested a review from Siilwyn March 9, 2023 13:27
Copy link
Member

@Siilwyn Siilwyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvements! Once you're back from vacation let's chat about this en er een klap op geven :)
Maybe in another PR we can add some docs about pulling env vars from Netlify since we're using netlify dev now...

.gitignore Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants